Skip to content

[wip] PatchedArray wiring: Part 2#7145

Draft
a10y wants to merge 3 commits intodevelopfrom
aduffy/lazy-patches
Draft

[wip] PatchedArray wiring: Part 2#7145
a10y wants to merge 3 commits intodevelopfrom
aduffy/lazy-patches

Conversation

@a10y
Copy link
Copy Markdown
Contributor

@a10y a10y commented Mar 24, 2026 •
edited
Loading

Summary

Part of #6535 epic and successor to #7002

This wires PatchedArray through to BitPackedArray, replacing the interior Patches entirely. This change is backward compatible.

API Changes

  1. Updates VTable::build to return ArrayRef instead of V::Array. This allows us to lazily deserialize a previously written value as a new type in a backward-compatible way
  2. Adds a new LazyPatchedArray which is just a dummy node that holds on to an array and some Patches, has no reductions and can just be executed into a transposed PatchedArray
  3. Removes the patches child from BitPackedArray
  4. Removes the BitPackedArray::encode() and a mess of other functions, instead moving everything into a new BitPackedEncoder fluent API

Testing

Reuses our existing and extensive test suite for BP

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 24, 2026 •
edited
Loading

Merging this PR will degrade performance by 20.33%

❌ 4 regressed benchmarks
✅ 424 untouched benchmarks
⏩ 2200 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation compress_rd[f32, 10000] 251.4 µs 310.7 µs -19.08%
❌ Simulation compress_rd[f32, 100000] 2.3 ms 2.9 ms -20.33%
❌ Simulation compress_rd[f64, 10000] 312.5 µs 377.2 µs -17.16%
❌ Simulation compress_rd[f64, 100000] 2.9 ms 3.5 ms -17.08%

Comparing aduffy/lazy-patches (595fd92) with develop (fcfd443)

Open in CodSpeed

Footnotes

  1. 2200 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@a10y a10y force-pushed the aduffy/lazy-patches branch 2 times, most recently from 1d28e78 to 65d8c10 Compare March 24, 2026 19:06
@a10y a10y force-pushed the aduffy/patched-array branch from 860cc64 to 042394b Compare March 24, 2026 19:17
@a10y a10y force-pushed the aduffy/lazy-patches branch 3 times, most recently from c76f6bd to 93e2785 Compare March 24, 2026 19:30
@a10y a10y changed the title [wip] BP remove interior Patches [wip] PatchedArray wiring: Part 2 Mar 24, 2026
@a10y a10y force-pushed the aduffy/lazy-patches branch from 93e2785 to dd20d70 Compare March 24, 2026 19:37
@a10y a10y force-pushed the aduffy/patched-array branch from 042394b to aa8f05f Compare March 24, 2026 19:48
@a10y a10y force-pushed the aduffy/lazy-patches branch 2 times, most recently from b7c14f1 to 69ce351 Compare March 25, 2026 20:26
@a10y a10y force-pushed the aduffy/patched-array branch 2 times, most recently from df8dbf5 to 862d814 Compare March 26, 2026 19:54
@a10y a10y force-pushed the aduffy/lazy-patches branch 2 times, most recently from 0d703d8 to e658811 Compare March 26, 2026 20:59
@a10y a10y force-pushed the aduffy/patched-array branch 4 times, most recently from ad0eaa7 to f3d9155 Compare March 31, 2026 16:56
Base automatically changed from aduffy/patched-array to develop March 31, 2026 18:39
@a10y a10y force-pushed the aduffy/lazy-patches branch from e658811 to c709a61 Compare April 1, 2026 16:27
a10y added 2 commits April 1, 2026 12:27
@a10y
add a LazyPatchedArray
44fbb7f 
this lets us deserialize BPArray with Patches without eagerly
transposing

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y
update VTable::build to return ArrayRef
e8e4a97 
This lets us return something other than the original array encoding at
read time.

Currently we'll want this so that BitPacked::build returns a
LazyPatched, but this is applicable for pretty much any back-compat
preserving encoding rewrites.

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y force-pushed the aduffy/lazy-patches branch from c709a61 to 786b70c Compare April 1, 2026 18:34
@a10y a10y added the feature A feature request label Apr 1, 2026
a10y
a10y commented Apr 1, 2026
View reviewed changes
vortex-cuda/src/dynamic_dispatch/mod.rs
}
}

#[cfg(test)]
Copy link
Copy Markdown
Contributor Author

@a10y a10y Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: add these back

@a10y
Remove patches from BitPackedArray
595fd92 
removes both the patches field as well as all code for handling patches.
this is safe to do now that we have updated the VTable build function to
always read methods.

note that we need to leave the metadata as-is.

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y force-pushed the aduffy/lazy-patches branch from 786b70c to 595fd92 Compare April 1, 2026 18:40
@a10y
Copy link
Copy Markdown
Contributor Author

a10y commented Apr 2, 2026

With current state of branch and PatchedArray, it looks like externalizing the patches adds ~1-2% to most of the ClickBench files.

image

At lot of that is due to wasted space in the lane_offsets where there are many leading zeros.

That would compress easily. The problem with compressing these is that I think we then lose ability to decode in a single kernel launch.

@a10y
Copy link
Copy Markdown
Contributor Author

a10y commented Apr 2, 2026 •
edited
Loading

We could add a LeadingZeros encoding which has a GPU implementation that does not launch any kernels, just orchestrates a calloc + memcpy on the GPU.

EDIT: this doesn't actually save a kernel, under the hood cudaMemset is just doing a kernel launch apparently.

joseph-isaacs
joseph-isaacs reviewed Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do one array at a time?

encodings/fastlanes/src/bitpacking/vtable/mod.rs
Comment on lines 286 to +289
) -> VortexResult<Option<ArrayRef>> {
PARENT_KERNELS.execute(array, parent, child_idx, ctx)
}

Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could add a execute_parent kernel that fuses BitPacked decompress and patch application

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joseph-isaacs joseph-isaacs joseph-isaacs left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

feature A feature request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@a10y @joseph-isaacs